feat(instrumentation-http): provide http.request.header.<key> at server span creation time#6396
Conversation
…an creation time Signed-off-by: Vitor Vasconcellos <vitor.vasconcellos@mercadolivre.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6396 +/- ##
=======================================
Coverage 95.68% 95.68%
=======================================
Files 364 364
Lines 11786 11786
Branches 2744 2744
=======================================
Hits 11278 11278
Misses 508 508 🚀 New features to boost your workflow:
|
Signed-off-by: Vitor Vasconcellos <vitor.vasconcellos@mercadolivre.com>
Signed-off-by: Vitor Vasconcellos <vitor.vasconcellos@mercadolivre.com>
trentm
left a comment
trentm
left a comment
There was a problem hiding this comment.
Nice! Thanks for taking this. This LGTM, just the changelog comment.
| ### :house: Internal | ||
|
|
||
| * perf(otlp-transformer): optimize toAnyValue performance [#6287](https://github.com/open-telemetry/opentelemetry-js/pull/6287) @AbhiPrasad | ||
| * chore(instrumentation-http): provide `http.request.header.<key>` at server span creation time [#6396](https://github.com/open-telemetry/opentelemetry-js/pull/6396) @vitorvasc |
There was a problem hiding this comment.
I'd call this a "feat" (also update the PR title), and move this to the "Features" section of the changelog. Also, because I've been slow to review and there has been a release in the interim, you'll need to move this up to the "# Unreleased" section.
There was a problem hiding this comment.
That makes sense, thanks!
Updated. 🙂
Signed-off-by: Vitor Vasconcellos <vitor.vasconcellos@mercadolivre.com>
vitorvasc
changed the title
http.request.header.<key> at server span creation timehttp.request.header.<key> at server span creation time
Signed-off-by: Vitor Vasconcellos <vitor.vasconcellos@mercadolivre.com>
There was a problem hiding this comment.
Pull request overview
Ensures http.request.header.<key> attributes for incoming (server) HTTP spans are available at span creation time, aligning with the HTTP semconv “SHOULD be provided at span creation time” guidance to support sampling decisions.
Changes:
- Refactors
headerCaptureto return anAttributesobject rather than mutating a span directly. - Moves server request header capture to occur before
_startHttpSpanby merging captured headers intospanOptions.attributes. - Updates remaining header capture call sites to apply returned attributes via
span.setAttributes(), and updates unit tests accordingly.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| experimental/packages/opentelemetry-instrumentation-http/test/functionals/utils.test.ts | Updates tests to validate returned header attributes instead of mocking span.setAttribute. |
| experimental/packages/opentelemetry-instrumentation-http/src/utils.ts | Refactors headerCapture to build and return an Attributes map. |
| experimental/packages/opentelemetry-instrumentation-http/src/http.ts | Applies captured headers via setAttributes() and captures server request headers before span creation. |
| experimental/CHANGELOG.md | Adds a changelog entry for the behavior change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Object.assign( | ||
| spanAttributes, | ||
| instrumentation._headerCapture.server.captureRequestHeaders( | ||
| header => request.headers[header] | ||
| ) | ||
| ); |
There was a problem hiding this comment.
New behavior moves server request header capture into the span creation attributes so sampling can use http.request.header.*. There isn't a test that verifies these header attributes are visible to the sampler at startSpan time (e.g., via a custom Sampler's shouldSample receiving them). Consider adding a functional/unit test that installs a Sampler capturing the attributes argument and asserts the configured header keys/values are present when the server span is created.
… attribute capturing Signed-off-by: Vitor Vasconcellos <vitor.vasconcellos@mercadolivre.com>
Which problem is this PR solving?
The HTTP semconv spec states that
http.request.header.<key>SHOULD be provided at span creation time, as it can be important for making sampling decisions. The current behavior is thatinstrumentation-httponly captures server hequest headers after span creation inside thecontext.with()callback.Fixes #6393
Short description of the changes
headerCaptureinutils.tsto return anAttributesobject instead of callingspan.setAttribute()directly.http.tsto before_startHttpSpan, merging the captured attributes intospanOptions.attributesso they are available at span creation time.span.setAttributes()with the returned attributes.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
npm run testin@opentelemetry/instrumentation-http)npm run lint)Checklist: